Hey! The outputFormatter function does some additional formatting I guess you can test it adding the &response_format=compact as far as I remember. Please make sure it returns the exact same format for the cached and non-cached responses. We need to fix it in storefront-api as well

@pkarw I'm already using the compact format and this leads me into the exception above, once I enable the outputCache.

As as I see the processor for the product entity is handling the "decompact" for the response properties in compactItem() of src/processor/product.js. And _outputFormatter cleans up the general request.

Okay if I compare the output of both requests you can see, the mutated one is landing in the output-cache.

First request – uncached:

{
  took: 4,
  timed_out: false,
  _shards: { total: 5, successful: 5, skipped: 0, failed: 0 },
  hits: { total: 414, max_score: null, hits: [ [Object] ] }
}

Second request – cached:

{
  hits: [ [Object] ],
  total: 414
}

Could it be a async problem with the process of _cacheStorageHandler, so that the _outputFormatter overwrites the _resBody variable before it is set into output cache and so the formatted version is saved to the cache?

Yep, that seems to be it. If I make _cacheStorageHandler asynchronous and await for it, it works.

Would it be better to wait for the output-cache to be written or clone the variable to not slow down the request in case the output-cache Redis or Varnish is blocking the request? – I would choose to create a new clone of the request variable.

Hi Christian, good point! I guess that results in cache should be post-processed by the formatted (to just serve the result HTML in case of Hit). I guess it won't have so much effect (either way you implement it) - so that's fine.

By the way, @cewald would you mind joining the storefront-api core-dev team? I really appreciate your PRs so far. There is @ResuBaka and @gibkigonzo already in the team. That would be awesome as it will replace vue-storefront-api at VSF 1.13 (second half of 2020)

I've updated the code.

I now pass the _resBody as a new variable to _outputFormatter. This way the original variable won't be mutated and saved into the output-cache.

I still marked the _cacheStorageHandler method as async to be aware that this is an asynchronous process (typewise).

@pkarw I can add this for storefront-api too, once approved, if you like. And sure, I would like to join if you need help :) I've planned to implement the storefront-api into my projects soon, so I'd be more in contact with it anyway.

storefront-api won't need an update for that as the _outputFormatter is called before the _cacheStorageHandler, and therefore the formatted version always will be in cache and returned.

So we would have to make await in line 156 and in line 151 to be sure that we put in cache always same resBody. Thats not what we want, because we don't want to wait for it. I think that better way is make same aproach as is in storefront-api. Format data before adding to cache. Wdyt?

Yes, we dont need/should to add await for it as we dont need to do anything synchronous after it, indeed. I just added the async to be aware that this is an async function - I can remove it again, but it shouldn‘t affect anything.

And to be sure to not save the mutatet version I cloned the reqBody variable using Object.assign().

But yes, we could do it the same as in the storefront-api and save always the mutated body. Should I change it as it is there?

Yes, please change this as it is in storefront-api. But keep async word for _cacheStorageHandler and return cache.set(. It's good to have in mind that this is asynchronous 😅

Okay, done. I also consolidated the double res.json(_resBody) of the if/else into one as it was equal in the end. Oh and my linter gone a bit wild and also removed some unnecessary semi's.